-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ui-alerts): trigger onDismiss for SR-only alerts #1718
fix(ui-alerts): trigger onDismiss for SR-only alerts #1718
Conversation
Thanks for the contribution, we will check it out soon |
it works great, thanks for the fix! Can you please squash the commits into one so there is just one with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, yes please squash the commits first.
5fdd33e
to
c4f3328
Compare
@matyasf @joyenjoyer Squashed into a single commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we will merge and release it soon :)
Thanks for the contribution, we will release this soon! |
onDismiss
was not being fired onscreenReaderOnly
alerts, unlesstransition: 'none'
was specified alongside.Default transition value is
'fade'
. This change makesscreenReaderOnly
alerts an implicittransition: 'none'
when executingonDismiss
.Credits for finding this bug: @meister245